Change how enums are defined. #3972
Conversation
There was a problem hiding this comment.
Looks good. Comments fall into a few buckets:
- Very minor formatting nitpicks that you can ignore if you want
- Marking unused symbols that we should get rid of in new commits (either in this PR or in one that follows it)
- One commit message typo, and one line in wrong commit
- One set of uses of
UINT64_C()that should beUINT32_C() - Noting where these changes alter the public API -- we can consider "API" commits, but that's up to you since this shouldn't meaningfully change anything.
| * dropped once all remote connections disconnect. | ||
| */ | ||
| lrmd_opt_drop_recurring = (1 << 0), | ||
| lrmd_opt_drop_recurring = (UINT32_C(1) << 0), |
There was a problem hiding this comment.
technically changes public API
| // NOTE: sbd (as of at least 1.5.2) uses this | ||
| //! \deprecated Do not use | ||
| #define pe_rsc_managed (1ULL << 1) | ||
| #define pe_rsc_managed (UINT64_C(1) << 1) |
There was a problem hiding this comment.
technically changes public API
There was a problem hiding this comment.
I think this one, at least, is totally fine. From what I can find, C99 guarantees that an unsigned long long is at least 64-bits. I don't know that we support any platforms where it would be 128-bits. Maybe there's a RISC-V platform that does this?
On the other hand, it's in a file that ends with _compat.h...
...to use the UINT64_C style of declaration. Ref T222
...to use the UINT64_C style of declaration. Ref T222
...to use the UINT64_C style of declaration. This also requires some other source modifications to use the PRIx64 format specifiers so everything still builds fine. Ref T222
...to use the UINT64_C style of declaration. This also requires some other source modifications to use the PRIx64 format specifiers so everything still builds fine. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT64_C style of declaration. Ref T222
...to use the UINT64_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. Ref T222
...to use the UINT32_C style of declaration. This isn't strictly necessary since they are deprecated, but it's nice to get them out of search results. Ref T222
clumens
force-pushed
the
enum-refactor
branch
from
c5f58f0 to
1d0b135
Compare
|
Rebased on main, no other changes yet. |
|
Regarding the ones that change public API... I can go either way on it. On the one hand, it's nice to get all these things changed over so we can eventually close out the upstream issue and I am pretty sure that all of these changes are okay. I can't find any evidence that gcc will make an enum anything other than 32-bits unless you go out of your way to force it. On the other hand, we tend to be pretty hesitant to change anything that's at all public, especially just for refactoring purposes. There's non-zero risk for basically zero reward. |
I'm not saying we need to avoid the changes, but rather that we may want to use "API:" commit messages. We've done so when changing public function signatures to use |
2 participants
Here's something really tedious. I think I got all of them, but there's various forms they can take so it's possible something got through. Also, just for the sake of full disclosure, I used a coding assistant for this (ugh) but did build and commit everything manually. It's possible I missed something really dumb that it did.